Skip to content

Auth/PM-3813 - 2FA Management Endpoints - User Verification Refactor#21385

Open
JaredSnider-Bitwarden wants to merge 31 commits into
mainfrom
auth/pm-38137/2fa-verification-refactor
Open

Auth/PM-3813 - 2FA Management Endpoints - User Verification Refactor#21385
JaredSnider-Bitwarden wants to merge 31 commits into
mainfrom
auth/pm-38137/2fa-verification-refactor

Conversation

@JaredSnider-Bitwarden

@JaredSnider-Bitwarden JaredSnider-Bitwarden commented Jun 19, 2026

Copy link
Copy Markdown
Contributor

🎟️ Tracking

https://bitwarden.atlassian.net/browse/PM-38137
Server PR: bitwarden/server#7841

📔 Objective

Refactors the web client's 2FA setup flow around a user-verification tokenable threaded from each per-provider GET through the subsequent PUT/DELETE. Migrates from the global PUT /two-factor/disable to per-provider DELETE endpoints (including a new DELETE /two-factor/webauthn/all for the bulk WebAuthn path), splits request/response models per action (Get / Update / Delete) with explicit *Details embedding, and reorganizes 2FA code into a libs/common/src/auth/two-factor/ feature folder with barrel exports.

Detailed Changes

Feature folder layout

All 2FA request DTOs, response models, types, and services moved under libs/common/src/auth/two-factor/ (subfolders request/, response/, types/). Each subfolder has an index.ts barrel; consumers import @bitwarden/common/auth/two-factor.

Request DTOs — libs/common/src/auth/two-factor/request/

All constructor-parameter classes, all standalone (no SecretVerificationRequest inheritance — no inherited masterPasswordHash on setup PUT/DELETE wire). All carry userVerificationToken; per-provider Delete DTOs carry only the token since the URL implies the provider type.

  1. AuthenticatorTwoFactorAuthenticatorUpdateRequest(token, key, uvToken), TwoFactorAuthenticatorDeleteRequest(key, uvToken). Renamed from legacy UpdateTwoFactorAuthenticatorRequest / DeleteTwoFactorAuthenticatorRequest to align with sibling TwoFactor<Provider><Verb> shape.
  2. DuoTwoFactorDuoUpdateRequest(clientId, clientSecret, host, uvToken), TwoFactorDuoDeleteRequest(uvToken), TwoFactorOrganizationDuoDeleteRequest(uvToken). Personal and org Duo share the Update DTO; routes diverge in the service.
  3. YubiKeyTwoFactorYubiKeyUpdateRequest(key1..key5, nfc, uvToken), TwoFactorYubiKeyDeleteRequest(uvToken). Class casing aligned to YubiKey enum value (was Yubico).
  4. Email — split three ways to mirror the server: TwoFactorEmailLoginRequest (anonymous /send-email-login, still secret-based; renamed from TwoFactorEmailRequest; dropped unused userVerificationToken? field), TwoFactorEmailSetupRequest (authenticated /send-email, token-only), TwoFactorEmailUpdateRequest (PUT /email, adds OTP token), TwoFactorEmailDeleteRequest (DELETE /email).
  5. WebAuthnTwoFactorWebAuthnUpdateRequest, TwoFactorWebAuthnDeleteRequest(id, uvToken), TwoFactorWebAuthnDeleteAllRequest(uvToken) (new bulk endpoint — per-credential delete refuses the last credential server-side, so bulk is the only path that disables the whole provider), TwoFactorWebAuthnChallengeRequest(uvToken) (new — challenge POST now replays the cached UV token from the prior GetWebAuthn instead of re-prompting).
Response models — libs/common/src/auth/two-factor/response/

Each provider gets a *Details class with its config fields plus per-action wrappers (Get embeds Details + userVerificationToken; Update / Delete embed Details only — PUT/DELETE responses don't re-mint a token).

  1. Per-provider triples (new) — Authenticator, Duo (user + org share TwoFactorDuoDetailsResponse), YubiKey, Email, WebAuthn each have a *Details + Get + Update response; WebAuthn additionally has a Delete response for per-credential remove. *Details files are renamed from the old models/response/two-factor-<provider>.response.ts shapes; per-action wrappers are net-new.
  2. two-factor-web-authn-challenge.response.ts (new) — thin { options: WebAuthnChallengeResponse | null } wrapper. UV token removed from this response in [279e72b1]; client now reuses the cached token from GetWebAuthn.
  3. two-factor-provider.response.ts, two-factor-recover.response.ts — pure file-move from models/response/. Shapes unchanged.
Shared WebAuthn challenge response — libs/common/src/auth/models/response/web-authn-challenge.response.ts
  1. Renamed ChallengeResponseWebAuthnChallengeResponse and moved out of the 2FA-specific path. Implements PublicKeyCredentialCreationOptions. Reused by both 2FA setup (wrapped in TwoFactorWebAuthnChallengeResponse) and passkey login (wrapped in WebauthnLoginCredentialCreateOptionsResponse).
  2. apps/web/src/app/auth/core/services/webauthn-login/response/webauthn-login-credential-create-options.response.ts + …/views/credential-create-options.view.ts — picked up the rename.
Types — libs/common/src/auth/two-factor/types/
  1. two-factor-user-verification-result.ts (new — replaces old auth/types/auth-response.ts){ secret, verificationType } returned by TwoFactorVerifyDialogComponent. Renamed from AuthResponse / AuthResponseBase to a 2FA-scoped name.
  2. two-factor-setup-dialog-data.ts (new)TwoFactorSetupDialogData<T extends TwoFactorResponse> = TwoFactorUserVerificationResult & { response: T }. The DIALOG_DATA shape passed into per-provider setup dialogs.
  3. two-factor-response.ts — moved from auth/types/. Union of all per-provider GET response types; used as the T constraint above.
Service layer — libs/common/src/auth/two-factor/{abstractions,services}/

two-factor-api.service.ts + default-two-factor-api.service.ts

  1. Added five per-provider DELETE methods (deleteTwoFactor{YubiKey,Duo,OrganizationDuo,Email,WebAuthnAll}), all returning Promise<void>.
  2. Removed putTwoFactorDisable / putTwoFactorOrganizationDisable — corresponding server endpoints are gone.
  3. getTwoFactorWebAuthnChallenge now takes TwoFactorWebAuthnChallengeRequest (UV token in body) and returns TwoFactorWebAuthnChallengeResponse.
  4. PUT return types narrowed to per-action *UpdateResponse wrappers (e.g., TwoFactorAuthenticatorUpdateResponse).
  5. postTwoFactorEmailSetup takes TwoFactorEmailSetupRequest; postTwoFactorEmail takes TwoFactorEmailLoginRequest (email-split mirroring).

two-factor.service.ts + default-two-factor.service.ts — mirrors the api-service surface (components consume this layer). Inlined the single-line processUpdateResponse / processDeleteResponse pass-throughs.

default-two-factor-api.service.spec.ts — rewritten against the new DTOs + wrappers; sections grouped by provider; covers the five new deleteTwoFactor* methods, the WebAuthn-challenge token-replay variant, and the email-split.

Web 2FA setup components — apps/web/src/app/auth/settings/two-factor/

two-factor-setup-method-base.component.ts

  1. disableMethod() is now protected abstract — the old shared implementation posted to the removed PUT /two-factor/disable. Each subclass routes to its own per-provider DELETE.
  2. auth(...) argument retyped to TwoFactorUserVerificationResult.
  3. TODO added (PM-39385) to remove the directive-base inheritance in favor of composition.

two-factor-setup-{authenticator,duo,email,yubikey,webauthn}.component.ts

  1. Each component caches userVerificationToken from the initial GET and threads it into enable PUT and disable DELETE — passwordless users complete setup with a single OTP. processResponse guards the assignment with if (response.userVerificationToken) so PUT responses (which don't re-mint) can't clobber the cached token (regression-guard for a same-session enable→disable bug caught in review on YubiKey, applied defensively to the others).
  2. Direct-instantiation of standalone request DTOs (no buildRequestModel); each disableMethod() override routes to the matching deleteTwoFactor<Provider>.
  3. Duo branches by organizationId between personal and org delete service methods.
  4. WebAuthn "Disable All Keys" calls the new deleteTwoFactorWebAuthnAll; per-credential remove still uses deleteTwoFactorWebAuthn; challenge POST constructs TwoFactorWebAuthnChallengeRequest with the cached UV token.
  5. Email setup uses TwoFactorEmailSetupRequest for postTwoFactorEmailSetup.
  6. Renamed response-processing helpers processXxxStateapplyXxxDetails and prefixed the param with the provider name for readability.

two-factor-setup.component.ts + two-factor-setup.component.html

  1. Preserved the lapsed-premium one-click escape hatch (red "Disable" button on the settings list). disablePremium2faTypeForNonPremiumUser was rewired from the removed PUT /two-factor/disable to a GET → DELETE chain inside the UV verificationFn (one UV dialog, two round-trips, identical UX).
  2. Kept the [disabled]="!(canAccessPremium$ | async) && p.premium" conditional on the Manage button for unenrolled premium providers.

two-factor-verify.component.ts — returns TwoFactorUserVerificationResult (renamed type); behavior unchanged.

Cross-app consumers
  1. apps/cli/src/auth/commands/login.command.ts + libs/auth/src/angular/two-factor-auth/child-components/two-factor-auth-email/two-factor-auth-email.component.ts — picked up TwoFactorEmailRequestTwoFactorEmailLoginRequest rename. Name-only edit; wire shape unchanged.
  2. apps/web/src/app/admin-console/organizations/settings/two-factor-setup.component.ts, …/two-factor/two-factor-recovery.component.ts, …/account/change-email.component.spec.ts — import-path updates for moved 2FA types. Behavior unchanged.
Removed legacy files
  1. libs/common/src/auth/models/request/disable-two-factor-authenticator.request.ts, two-factor-provider.request.ts (no remaining consumers), and the six update-two-factor-*.request.ts files. All replaced by their TwoFactor<Provider>{Update,Delete}Request counterparts in the feature folder.
  2. libs/common/src/auth/types/auth-response.ts (replaced by TwoFactorUserVerificationResult + TwoFactorSetupDialogData) and two-factor-response.ts (moved into the feature folder).

📸 Screenshots

A. Web — Master-password user — end-to-end tests per 2FA provider

A1 — Email
PM-38137.-.A1.-.Web.-.Password.user.-.Email.2FA.end-to-end.mov
A2 — Authenticator
PM-38137.-.A2.-.Web.-.Password.user.-.Authenticator.2FA.end-to-end.mov
A3 — WebAuthn
PM-38137.-.A3.-.Web.-.Password.user.-.WebAuthn.2FA.end-to-end.mov
A4 — YubiKey
PM-38137.-.A4.-.Web.-.Password.user.-.YubiKey.2FA.end-to-end.mov
A5 — Duo (personal)

See Jira.

A6 — OrgDuo

See Jira.

B. Web — Passwordless user — end-to-end tests per 2FA provider

B1 — Email
PM-38137.-.B1.-.Web.-.Passwordless.user.SSO+TDE.-.Email.2FA.end-to-end.mov
B2 — Authenticator
PM-38137.-.B2.-.Web.-.Passwordless.user.SSO+TDE.-.Authenticator.2FA.end-to-end.mov
B3 — WebAuthn
PM-38137.-.B3.-.Web.-.Passwordless.user.SSO+TDE.-.WebAuthn.2FA.end-to-end.mov
B4 — YubiKey
PM-38137.-.B4.-.Web.-.Passwordless.user.SSO+TDE.-.YubiKey.2FA.end-to-end.mov
B5 — Duo personal

See Jira.

C. Web — Lapsed-premium UX

C1 — YubiKey one-click Disable
PM-38137.-.C1.-.Web.-.Lapsed-premium.user.-.YubiKey.one-click.Disable.mov
C2 — Duo one-click Disable
PM-38137.-.C2.-.Web.-.Lapsed-premium.user.-.Duo.one-click.Disable.mov

D. Web — Provider-specific UX

D1 — WebAuthn key removal flow
PM-38137.-.D1.-.Web.-.WebAuthn.2FA.key.removal.flow.mov
D2 — YubiKey same-session enable then disable
PM-38137.-.D2.-.Web.-.YubiKey.2FA.enable.then.disable.in.same.dialog.session.mov

E. Web — UX recovery

E1 — Token expiry (temp hard coded to 15 s on server)
PM-38137.-.E1.-.Web.-.Password.user.-.UV.token.expiry.15.second.expiration.hardcoded.on.server.mov
E2 — Passwordless invalid OTP
PM-38137.-.E2.-.Web.-.Passwordless.user.SSO+TDE.-.Invalid.OTP.UX.mov

F. CLI

F1 — Login with email 2FA
PM-38137.-.F1.-.CLI.-.Password.user.-.Login.with.email.2FA.mov

Mirror the server-side per-provider model rewrite on the client:

- Existing PUT/DELETE setup request models drop SecretVerificationRequest
  inheritance and become standalone with explicit userVerificationToken
  fields. two-factor-email.request keeps its inheritance because it also
  serves the login flow.
- New per-provider delete request models (YubiKey, Duo, Email,
  OrganizationDuo, WebAuthn delete-all) — token-only shape mirroring
  the server.
- Authenticator delete request file/class renamed from
  disable-two-factor-authenticator to delete-two-factor-authenticator
  to match server naming.
- New TwoFactorWebAuthnChallengeResponse wrapper around the FIDO2
  options + minted token (replaces the bare ChallengeResponse payload
  from get-webauthn-challenge).
- Response models for Duo, Email, WebAuthn, and YubiKey gain
  userVerificationToken.
- Obsolete two-factor-provider.request deleted (no remaining consumers
  after the disable-model rewrite).
Both TwoFactorApiService and TwoFactorService (abstractions and
implementations) pick up:

- New methods deleteTwoFactorYubiKey, deleteTwoFactorDuo,
  deleteTwoFactorEmail, deleteTwoFactorOrganizationDuo, and
  deleteTwoFactorWebAuthnAll routing to the corresponding per-provider
  DELETE endpoints.
- Removal of legacy putTwoFactorDisable and
  putTwoFactorOrganizationDisable (server endpoints are gone).
- Updated return type on getTwoFactorWebAuthnChallenge to the new
  TwoFactorWebAuthnChallengeResponse wrapper.

DefaultTwoFactorApiService spec: legacy put*Disable tests removed,
five new deleteTwoFactor* tests added, WebAuthn challenge test asserts
the new wrapper.
Per-provider setup components (Authenticator, YubiKey, Duo, Email,
WebAuthn) instantiate request models directly, cache the user
verification token from the GET response, and thread it through every
PUT / DELETE / setup-POST call. Each component implements its own
disableMethod against the appropriate per-provider DELETE endpoint:

- Authenticator, YubiKey, Email each call their corresponding
  deleteTwoFactor* method.
- Duo branches on organizationId between deleteTwoFactorDuo and
  deleteTwoFactorOrganizationDuo (the component is shared between
  personal Duo and OrgDuo setup).
- WebAuthn's "Disable All Keys" button calls
  deleteTwoFactorWebAuthnAll (single round-trip; server-side handles
  the wipe atomically). Per-credential remove continues to use
  deleteTwoFactorWebAuthn.
- WebAuthn challenge consumer reads options + userVerificationToken
  from the new wrapper response.

Base setup component disableMethod becomes protected abstract — every
subclass provides its own override. Parent settings page stops
rendering the lapsed-premium-only secondary "Disable" button; the
standard "Manage" button is now enabled for lapsed-premium users on
already-enrolled premium providers, so the same GET → DELETE flow
handles them.
@JaredSnider-Bitwarden JaredSnider-Bitwarden added the ai-review Request a Claude code review label Jun 19, 2026
@github-actions

github-actions Bot commented Jun 19, 2026

Copy link
Copy Markdown
Contributor

🤖 Bitwarden Claude Code Review

Overall Assessment: APPROVE

Reviewed this 2FA management refactor that threads a user-verification token from each per-provider GET through the subsequent PUT/DELETE, migrates from the global PUT /two-factor/disable to per-provider DELETE endpoints, splits request/response models per action, and reorganizes the code into the libs/common/src/auth/two-factor/ feature folder. Focused on the security-sensitive token-threading paths, the WebAuthn challenge token replay, the lapsed-premium GET→DELETE escape hatch, and breaking-change hygiene for the renames and file moves.

Code Review Details

No new findings.

  • The UV-token guard (requireUserVerificationToken() throwing when absent) is applied consistently across all five provider components; the authenticator gap previously flagged was already fixed in a8eba068f2.
  • PUT/DELETE response handlers consume only the *Details object, so they cannot clobber the cached GET token — the same-session enable→disable regression is avoided structurally.
  • No dangling references remain to any removed or renamed symbol (putTwoFactorDisable, ChallengeResponse, legacy request DTOs, old auth/types paths); old files are deleted and barrel exports follow the feature-folder convention.
  • The rewritten api-service spec covers GET token extraction, the five new per-provider DELETEs, the WebAuthn challenge token replay, and request-object non-mutation.

@codecov

codecov Bot commented Jun 19, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 45.85492% with 209 lines in your changes missing coverage. Please review.
✅ Project coverage is 49.33%. Comparing base (f011874) to head (a8eba06).
⚠️ Report is 10 commits behind head on main.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
.../two-factor/two-factor-setup-webauthn.component.ts 0.00% 38 Missing ⚠️
...s/two-factor/two-factor-setup-yubikey.component.ts 0.00% 30 Missing ⚠️
...tings/two-factor/two-factor-setup-duo.component.ts 0.00% 27 Missing ⚠️
...ngs/two-factor/two-factor-setup-email.component.ts 0.00% 26 Missing ⚠️
libs/common/src/auth/two-factor/response/index.ts 0.00% 21 Missing ⚠️
...factor/two-factor-setup-authenticator.component.ts 0.00% 15 Missing ⚠️
.../settings/two-factor/two-factor-setup.component.ts 0.00% 15 Missing ⚠️
libs/common/src/auth/two-factor/request/index.ts 0.00% 15 Missing ⚠️
.../two-factor/services/default-two-factor.service.ts 0.00% 5 Missing ⚠️
...o-factor/two-factor-setup-method-base.component.ts 0.00% 3 Missing ⚠️
... and 8 more
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #21385      +/-   ##
==========================================
- Coverage   49.36%   49.33%   -0.03%     
==========================================
  Files        4085     4120      +35     
  Lines      128461   128890     +429     
  Branches    19727    19763      +36     
==========================================
+ Hits        63410    63587     +177     
- Misses      60355    60603     +248     
- Partials     4696     4700       +4     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Restores the dedicated "Disable" button shown on the 2FA settings list
when a user has lost premium but still has an enrolled premium provider
(YubiKey or Duo), and restores the disabled "Manage" button for
unenrolled premium providers — i.e., the exact UX present before this
branch.

Under the hood the shortcut now uses the new per-provider DELETE
architecture: in the user-verification dialog's verificationFn the
component calls the per-provider GET (now non-premium-gated) to mint a
UV token, then the per-provider DELETE with that token. Two server
round-trips behind one UV dialog interaction.

Avoids the regression where the standard manage flow would have opened
the full provider configuration screen to a lapsed-premium user and
let them attempt to add more keys before failing at PUT-time on the
server.
…ate shape

Aligns every client 2FA update request-model class to the file-wide
TwoFactor<Provider> prefix already used by the Delete family:

  UpdateTwoFactorAuthenticatorRequest  -> TwoFactorAuthenticatorUpdateRequest
  UpdateTwoFactorDuoRequest            -> TwoFactorDuoUpdateRequest
  UpdateTwoFactorYubikeyOtpRequest     -> TwoFactorYubiKeyUpdateRequest
  UpdateTwoFactorEmailRequest          -> TwoFactorEmailUpdateRequest
  UpdateTwoFactorWebAuthnRequest       -> TwoFactorWebAuthnUpdateRequest
  UpdateTwoFactorWebAuthnDeleteRequest -> TwoFactorWebAuthnDeleteRequest

The YubiKey rename also aligns the class name with the TwoFactorProviderType
enum value (YubiKey, not YubicoOtp). No HTTP route or wire-shape changes.
The shared TwoFactorEmailRequest split into purpose-specific models:

- TwoFactorEmailLoginRequest (anonymous login flow, secret-based) drops
  the userVerificationToken field that the server-side login model no
  longer accepts.
- TwoFactorEmailSetupRequest (authenticated setup-send, token-only:
  email + userVerificationToken). postTwoFactorEmailSetup now takes this
  narrower shape instead of the union.
Only PUT responses that re-mint a user-verification token should
overwrite the cached value on the component. Without the guard, a PUT
response with a null token clobbers the live token from the prior GET,
breaking any follow-up DELETE that still runs against the same component
instance.

Matches the existing guard pattern in two-factor-setup-webauthn.component.ts.
@JaredSnider-Bitwarden JaredSnider-Bitwarden added the t:tech-debt Change Type - Tech debt label Jun 23, 2026
Convert the 13 per-provider 2FA request models added in this branch from
field-assignment classes (with `!` definite-assignment assertions) to
constructor-parameter classes. Callers now pass every required field at
construction; missing fields become compile-time errors instead of silent
`undefined` payloads.

Consumers updated:
- apps/web/src/app/auth/settings/two-factor/two-factor-setup-{authenticator,duo,email,webauthn,yubikey,}.component.ts
- libs/common/src/auth/two-factor/services/default-two-factor-api.service.spec.ts
Client mirror of the new server response shapes. Adds:

- TwoFactor<Provider>DetailsResponse — five per-provider Details types
  (Authenticator, Duo, Email, WebAuthn, YubiKey) carrying the shared
  provider state used by both GET and Update response wrappers.
- TwoFactor<Provider>UpdateResponse — five per-endpoint PUT responses,
  one per provider, each composing the matching Details type with no
  user-verification token slot.
- TwoFactorOrganizationDuoResponse / TwoFactorOrganizationDuoUpdateResponse —
  split from the user-scoped Duo wrappers so the two scopes can evolve
  independently while sharing the inner TwoFactorDuoDetails.
- TwoFactorWebAuthnDeleteResponse — returned by the per-credential
  WebAuthn DELETE; the updated credentials list travels in the body.

No service-surface or component wiring yet; those land in the next
commits.
…larity

The FIDO2 credential-creation options class previously lived inside
two-factor-web-authn.response.ts under the generic name
ChallengeResponse, which read like it could be any challenge response.
Moves the class to its own file (web-authn-challenge.response.ts) and
renames it to WebAuthnChallengeResponse so its scope is obvious at
every read site.

Touches the 2FA WebAuthn challenge wrapper (TwoFactorWebAuthnChallengeResponse)
and both passkey-login consumers (WebauthnLoginCredentialCreateOptionsResponse,
CredentialCreateOptionsView) to import the renamed type from its new
home.
…response types

Wires the client to the new per-action response shapes. The five
existing TwoFactor<Provider>Response classes are repurposed as the GET
response wrappers — each composes a TwoFactor<Provider>DetailsResponse
plus the freshly-minted user-verification token.

Service surface updated:
- DefaultTwoFactorApiService.getTwoFactor<Provider> returns the GET
  wrapper; put returns the matching *UpdateResponse.
- The six hard-delete methods return Promise<void> and pass
  hasResponse: false to ApiService.send — the matching server endpoints
  now return 204 No Content with no body. deleteTwoFactorWebAuthn
  (per-credential) keeps a body and returns TwoFactorWebAuthnDeleteResponse.
- TwoFactorService pass-through and abstractions updated to match.
- TwoFactorResponse discriminated union updated to reference the
  per-provider GET response classes (plus TwoFactorOrganizationDuoResponse).

Web setup components updated:
- processResponse split into per-action handlers (GET, Update, Delete
  where applicable) so each handler reads from the nested data
  property on its specific response type.
- The runtime `if (response.userVerificationToken)` cache guard from
  PM-38137-prior is dropped — the GET response type now guarantees the
  token non-optionally, and Update/Delete response types carry no token
  slot at all.
- DTO read sites updated from flat (`response.host`) to nested
  (`response.duo.host`) across all five providers.
- The admin-console organization parent setup component is updated for
  the new TwoFactorOrganizationDuoResponse name.
…older

Relocates 2FA request and response model files from the convention folders
(libs/common/src/auth/models/{request,response}/) into the two-factor feature
folder, per the libs/common/src/auth/CLAUDE.md rule that new auth code belongs
in feature folders. Adds request/ and response/ barrels and re-exports them
from the two-factor index. Updates consumer imports across web, cli,
libs/auth, and libs/common.

Leaves identity-two-factor.response.ts (login-flow response, identity-token
group) and web-authn-challenge.response.ts (shared with webauthn-login)
in place to avoid cross-feature coupling.
Renames DeleteTwoFactorAuthenticatorRequest to TwoFactorAuthenticatorDeleteRequest
(file: two-factor-authenticator-delete.request.ts) so the authenticator delete
request matches the TwoFactor<Provider>Delete shape used by the duo, email,
yubikey, web-authn, and organization-duo deletes.
…clearer names

Relocates the 2FA type aliases out of the convention folder
(libs/common/src/auth/types/) and into the two-factor feature folder per the
libs/common/src/auth/CLAUDE.md feature-folder rule. Renames the types so they
describe what they actually are: AuthResponseBase becomes
TwoFactorUserVerificationResult (the master-password/OTP verification proof
threaded into 2FA management request DTOs) and AuthResponse<T> becomes
TwoFactorSetupDialogData<T> (the payload passed as DIALOG_DATA into per-
provider 2FA setup dialogs). Consumers now import from the
@bitwarden/common/auth/two-factor barrel.
…am flow

TwoFactorUserVerificationResult feeds the initial GET / WebAuthn challenge POST
that mints the user-verification token, not the per-provider PUT/DELETE
endpoints — those take the cached UV token string directly. Also clarifies
that TwoFactorSetupDialogData<T> is the return type of TwoFactorVerifyDialog
before being forwarded as DIALOG_DATA to the per-provider setup dialogs.
Extracts TwoFactorUserVerificationResult from two-factor-setup-dialog-data.ts
into two-factor-user-verification-result.ts so each type lives in its own
file. Consumers continue to import from the @bitwarden/common/auth/two-factor
barrel and are unchanged.
…ification-refactor + merge conflicts in 2fa services
Restores relative imports inside the three two-factor API service files
to match the pattern that landed on main. Sibling two-factor service files
were already relative; these three drifted to absolute @bitwarden/common
paths during the refactor.
…nents

Replaces the definite-assignment assertion on the cached
userVerificationToken with an honest string | undefined type and a
requireUserVerificationToken() helper that throws if the field hasn't
been populated yet. Aligns with the typescript-strict ADR (!: is reserved
for required @input properties) and turns a silent undefined-into-request
hazard into an explicit runtime error. The base-class composition refactor
tracked by PM-39385 will eventually eliminate the cached field entirely.
The applyXxxState helpers in the Duo, Email, WebAuthn, and YubiKey setup
components took a TwoFactorXxxDetailsResponse but named the parameter after
the wrapper field (yubiKey, duo, emailData, webAuthn), which made accesses
like yubiKey.key1 read as if poking the wrapper rather than the details.
Renames the parameter to details across all four; the enclosing function
name already establishes which provider it is.
Renames the applyXxxState parameter from details to <provider>Details in
the Duo, Email, WebAuthn, and YubiKey setup components. Trades a few extra
characters per access for clearer reading at the call site
(e.g. yubiKeyDetails.key1 over details.key1).
…nents

The applyXxxState helpers project a per-provider details payload into
local form controls and display flags; "State" misleadingly suggested
writes to the app's state framework. Renames to applyXxxDetails across
authenticator, duo, email, webauthn, and yubikey setup components.
Comment thread apps/web/src/app/auth/settings/two-factor/two-factor-setup-webauthn.component.ts Outdated

@JaredSnider-Bitwarden JaredSnider-Bitwarden Jun 25, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

WebAuthn response model restructure

The single two-factor-web-authn.response.ts on main exported three classes; this branch splits them across six files to match the per-action wrapper pattern used by the other providers (Yubikey/Duo/Email/Authenticator).

Renames / relocations:

  • KeyResponseWebAuthnKeyResponse (was a too-generic top-level symbol). Now lives in two-factor-web-authn-details.response.ts alongside the TwoFactorWebAuthnDetailsResponse that embeds it.
  • ChallengeResponseWebAuthnChallengeResponse. Extracted into its own web-authn-challenge.response.ts (kept outside two-factor/) because the passkey-login flow shares it — moving it under two-factor/ would create a webauthn-login → two-factor feature dependency.
  • TwoFactorWebAuthnResponse survives, but its inline enabled / keys fields moved out into the new TwoFactorWebAuthnDetailsResponse. The wrapper now just holds webAuthn: TwoFactorWebAuthnDetailsResponse + userVerificationToken, mirroring the other providers.

New per-action wrappers (one per endpoint):

  • TwoFactorWebAuthnChallengeResponse — challenge POST. Carries only options: WebAuthnChallengeResponse | null. The challenge step replays the UV token minted by the WebAuthn GET (server-side TwoFactorUserVerificationTokenable validation), and that same token stays valid for the subsequent PUT — so no fresh token is echoed back. The matching request is the new TwoFactorWebAuthnChallengeRequest which carries just the UV token (replaces the old SecretVerificationRequest, fixing single-OTP enrollment for passwordless users).
  • TwoFactorWebAuthnUpdateResponse — PUT response, wraps refreshed Details.
  • TwoFactorWebAuthnDeleteResponse — per-credential and bulk DELETE responses, wraps refreshed Details.

Nothing was deleted; everything that used to live in the old single file is still reachable under a clearer name and location.

JaredSnider-Bitwarden and others added 5 commits June 24, 2026 20:22
Updates the per-provider delete docstrings on the 2FA service abstractions
and one inline WebAuthn comment that still described the operation as
"disabling the provider" — the operation is a hard delete, not a soft
disable. User-facing UI strings (the "Disable" button label, dialog
title) and method names that mirror that UI concept are unchanged.
Removes the processUpdateResponse and processDeleteResponse helpers
in the 5 2FA setup components — each body was a single-line wrapper
that called applyXxxDetails(response.xxx). Call sites now apply the
details directly, which makes the unwrap visible inline. The
processGetResponse helper stays because it bundles two coordinated
steps (cache user-verification token + apply details).

Also drops now-unused TwoFactor*UpdateResponse imports in each file.
…le sentences

Each 2FA wrapper response file had a docstring naming the specific server
endpoint that produced it (e.g. \`POST /two-factor/get-authenticator\`),
which breaks the moment the server renames an endpoint. Replaces with
one-sentence descriptions that describe what the response represents —
no endpoint coupling.
Switches the WebAuthn challenge client call from a single-use
SecretVerificationRequest to the new TwoFactorWebAuthnChallengeRequest
carrying the cached user-verification token minted by
getTwoFactorWebAuthn. Drops the now-stale userVerificationToken field
from TwoFactorWebAuthnChallengeResponse — the original token stays
valid through the subsequent PUT. Fixes the passwordless (TDE /
Key Connector) WebAuthn enrollment failure where the second use of
the OTP was rejected.
@JaredSnider-Bitwarden JaredSnider-Bitwarden marked this pull request as ready for review June 25, 2026 21:18
@JaredSnider-Bitwarden JaredSnider-Bitwarden requested review from a team as code owners June 25, 2026 21:18
Adds the requireUserVerificationToken() helper already present in the
duo/email/webauthn/yubikey setup components. Authenticator was the
only setup component still passing the raw string | undefined field
into request DTOs — silently allowed only because the file is
@ts-strict-ignore. The helper throws if the token wasn't populated,
keeping all five components consistent.
@sonarqubecloud

Copy link
Copy Markdown

@BTreston BTreston left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved for the AC owned file

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ai-review Request a Claude code review t:tech-debt Change Type - Tech debt

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants